Skip to content

feat(store): index-format rebuild boundary for the File-QN change#1104

Draft
LA-10 wants to merge 1 commit into
DeusData:mainfrom
LA-10:fix/769-file-qn-keeps-extension
Draft

feat(store): index-format rebuild boundary for the File-QN change#1104
LA-10 wants to merge 1 commit into
DeusData:mainfrom
LA-10:fix/769-file-qn-keeps-extension

Conversation

@LA-10

@LA-10 LA-10 commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

cbm_pipeline_fqn_compute() stripped the extension unconditionally. For sibling
component files that's the only distinguishing part, so badge.component.ts/.html/.scss
all produced the same __file__ QN and cbm_gbuf_upsert_node() merged them into one
File node. cbm_store_list_files() then returned 8 paths for 12 files, and
write_scoped_filelist() handed that short list to grep — so search_code never opened
the merged-away siblings and silently returned incomplete results.

Not Angular-specific: it's stem equality after one strip. spec.ts/stories.ts survive
because their stems differ. Same class as #495.

Fix: skip the strip for the __file__ sentinel. Symbol and module QNs unchanged.

Verification

Repro repo @ 5e39bf9, mode=full:

before after
File nodes / files 8 / 12 12 / 12
extension vs file_path mismatched matches
search_code markers 4/5 5/5

Also fixes the extension-corruption follow-up (pass_githistory upserts landing on the
merged node).

Known gaps

Module QNs still collide; File nodes still have start_line/end_line 0 so .scss matches
land in raw_matches. Happy to add repro_issue769.c for end-to-end search coverage if wanted.

@LA-10 LA-10 requested a review from DeusData as a code owner July 15, 2026 20:50
@LA-10 LA-10 force-pushed the fix/769-file-qn-keeps-extension branch from 4cb3014 to b79f397 Compare July 15, 2026 20:54
@DeusData DeusData added bug Something isn't working parsing/quality Graph extraction bugs, false positives, missing edges ux/behavior Display bugs, docs, adoption UX priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker. labels Jul 15, 2026
@DeusData DeusData added this to the 0.9.1-rc milestone Jul 15, 2026
@DeusData

Copy link
Copy Markdown
Owner

Thanks for the focused diagnosis and for linking the public #769 repro. I have triaged this as a high-priority 0.9.1-rc parsing/search correctness fix. The current diff has no dependency, workflow, network, credential, prompt-injection, or supply-chain concern.

The present unit coverage is not sufficient for this change:

  1. Angular component sibling files (ts/html/scss) merged into one File node; search_code silently incomplete #769's existing maintainer acceptance criterion requires an end-to-end red regression that asserts one File node per .ts/.html/.scss sibling and complete search_code marker coverage. Please add that repro test; simple FQN inequality does not prove the graph and scoped file list.
  2. Assert the exact new File QNs, not only pairwise inequality, and cover multi-extension names, extensionless files, and dotfiles.
  3. This changes the persisted qualified_name of every File node. An unchanged existing index can retain old identities, while an incremental update can introduce new identities only for touched files. That can create a mixed graph with duplicates, stale edges, or missing lookups. Please do not implement a migration or CI expansion yet; maintainers need to choose between an explicit full-reindex compatibility boundary and a migration/versioning strategy.
  4. Remove the trailing whitespace in fqn.c and the leading "# " from the PR title.

CI is still running. The identity compatibility decision remains a blocker even if it turns green.

@LA-10 LA-10 changed the title # fix(fqn): keep the extension in __file__ QNs so sibling files stay distinct Refs #769 fix(fqn): keep the extension in __file__ QNs so sibling files stay distinct Refs 769 Jul 15, 2026
@DeusData

Copy link
Copy Markdown
Owner

Maintainer decision: the File-QN direction is accepted, and we will use an automatic one-time full-reindex compatibility boundary rather than an in-place migration.

Please update this PR with the following scope:

  1. Add the end-to-end RED regression requested above for the Angular component sibling files (ts/html/scss) merged into one File node; search_code silently incomplete #769 sibling-file corpus. It must prove distinct File nodes and complete search_code marker coverage, then turn green with the fix.
  2. Add a narrow persisted index-format compatibility marker for this File-QN format change. A pre-change index must be routed through the existing full-reindex path before incremental or watcher refresh can produce mixed old/new File identities. A freshly rebuilt/current-format index must not rebuild again on the next unchanged run.
  3. Add a regression that starts from a pre-change index, proves the one-time full rebuild, verifies the distinct .ts, .html, and .scss File nodes afterward, and verifies that a subsequent run does not force another rebuild. Preserve the existing ADR/project metadata behavior of the full-reindex path.
  4. Keep this as a rebuild boundary; do not add an in-place node-renaming migration. Old collided databases no longer contain enough information to reconstruct every missing sibling safely.
  5. Retain the exact-QN and filename-edge-case coverage requested above and fix the current lint failure from trailing whitespace.

This resolves the maintainer-owned compatibility decision. Merge review will resume against the new exact head after these correctness and upgrade-path tests are present and CI is green.

@LA-10 LA-10 force-pushed the fix/769-file-qn-keeps-extension branch from 0d41961 to 4e18bf2 Compare July 16, 2026 13:00
…ary Refs DeusData#769

Signed-off-by: LA-10 <leenshuhail1@gmail.com>
@LA-10 LA-10 force-pushed the fix/769-file-qn-keeps-extension branch from 9408889 to 074edcc Compare July 16, 2026 14:12
@LA-10 LA-10 changed the title fix(fqn): keep the extension in __file__ QNs so sibling files stay distinct Refs 769 feat(store): index-format rebuild boundary for the File-QN change Jul 16, 2026
@LA-10

LA-10 commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto main. #1108 landed the same file guard while this was in review, so I’ve dropped my fqn.c change — this PR no longer touches that file.
What’s left is the compatibility boundary from your scope, which isn’t in #1108. So main currently has the gap you flagged: a pre-change index keeps collided File QNs, and the next incremental run mints new-format QNs only for touched files. This adds the PRAGMA user_version marker stale format routes through the existing full reindex path once, rebuilt index doesn’t rebuild again.
Worth flagging: the stamp has to be on both dump paths. cbm_writer_open truncates the DB file on every dump, so without re-stamping on the incremental path, every run resets the marker and forces a rebuild on the next. test_incremental.c’s noop/rapid tests caught it.
Also keeping repro_issue769.c for the search_code sibling coverage #1108’s tests don’t have.

@LA-10 LA-10 marked this pull request as draft July 16, 2026 15:45
@DeusData

Copy link
Copy Markdown
Owner

Thanks for rebasing this on top of #1108 and narrowing the PR to the compatibility boundary. That remaining boundary is useful, but this head is not ready to merge yet:

  • Please move or port the new end-to-end regressions into a gating CI suite. They are currently registered only under test-repro, so the green required checks do not execute them.
  • Please make the upgrade test represent an unmistakably legacy/collided graph. Building a current-format graph and only setting PRAGMA user_version = 0 proves the rebuild message, but not that the rebuild repairs old File identities. The test should prove the stale identity is removed, exact new QNs appear, ADR/project metadata survives, and a second unchanged run stays incremental.
  • A failed format-stamp write should fail the indexing operation. Logging and returning success leaves the database stale and can force a full rebuild on every run.
  • Please update the PR description to match the post-fix(fqn): preserve full filename in File-node QNs so sibling files don't collide #1108 scope and clean the trailing whitespace/final newline findings.

Once those are in place, we can re-review the exact updated head.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working parsing/quality Graph extraction bugs, false positives, missing edges priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker. ux/behavior Display bugs, docs, adoption UX

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants